Conversation
| if (name.length() > 5) { | ||
| throw new IllegalArgumentException(); | ||
| } |
There was a problem hiding this comment.
5 와 같이 무언가 의미있는 수들은 상수로 바꿔서 작성하시는 것을 권장드립니다
private static final int CAR_MOVE_STANDARD = 5;
There was a problem hiding this comment.
피드백 감사합니다. 아직은 상수 분리까지 익숙하지 않아서 노력해보겠습니다!
| throw new IllegalArgumentException(); | ||
| } | ||
|
|
||
| if (name.chars().anyMatch(Character::isWhitespace)) { |
There was a problem hiding this comment.
오 이런 식으로도 공백을 체크할 수 있군요..!
스트림을 활용하시는 것 좋습니다
There was a problem hiding this comment.
공백 검사할 방법을 찾아보다가 이렇게 써봤습니다. 그래도 자신감이 생기네요
| List<Car> cars = makeCars(); | ||
| int tryCount = getTryCount(); | ||
|
|
||
| System.out.println(); |
There was a problem hiding this comment.
System.out.print('\n') 으로 하는 것이 성능 상 이득을 볼 수 있습니다!
그 이유도 함께 조사해보시면 좋을 것 같아요
There was a problem hiding this comment.
제가 백슬래시 쓰는법을 몰라서 ㅠㅠ 조사하면서 배우겠습니다.
| try { | ||
| count = Integer.parseInt(input); | ||
| } catch (NumberFormatException e) { | ||
| throw new IllegalArgumentException(); | ||
| } |
There was a problem hiding this comment.
Integer.parseInt() 를 활용해서 숫자인지 체크하는 것 좋습니다
There was a problem hiding this comment.
제가 잘 몰라서 숫자인지 체크하는 이유가 뭔가요? 잘못된 입력을 처리하기 위해서인가요?
| for (Car car : cars) { | ||
| if (car.getPosition() > max) { | ||
| max = car.getPosition(); | ||
| } | ||
| } |
There was a problem hiding this comment.
이 코드의 depth 를 다음과 같이 바꿔서 한 단계 줄일 수도 있습니다
| for (Car car : cars) { | |
| if (car.getPosition() > max) { | |
| max = car.getPosition(); | |
| } | |
| } | |
| for (Car car : cars) { | |
| max = Math.max(max, car.getPosition()); | |
| } |
| public int getPosition() { | ||
| return position; | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
각 클래스 파일에는 맨 끝에 개행 한 줄을 추가해주시길 바랍니다!
자료 참고 부탁드립니다
|
|
||
| private static final int MAX_CAR_NAME_LENGTH = 5; | ||
| private static final int MIN_TRY_COUNT = 1; | ||
| private static final int RANDOM_START = 0; | ||
| private static final int RANDOM_END = 9; | ||
| private static final int MOVE_THRESHOLD = 4; | ||
|
|
There was a problem hiding this comment.
저번에 피드백 드렸던 부분을 잘 반영해주셨군요! 좋습니다
다만 MOVE_THRESHOLD 와 같은 경우에는,
Car 을 움직이지 말지에 대한 여부를 결정하는 로직이 Car 클래스의 move() 메소드에 있으니,
이 상수는 Car 로 옮겨도 될 것 같습니다
There was a problem hiding this comment.
현재 클래스의 크기가 좀 큰데,
관련 기능(메소드)들을 모아서 클래스를 여러 개 만들어서 분리하는 것도 좋을 것 같습니다!
현재 메소드의 각 역할들은 잘 분리해 주셨는데,
예를 들면, validateName() 처럼 Car에 대한 예외처리를 담당하는 메소드들은 CarValidator ,
createCars() 처럼 자동차 생성에 필요한 도구들은 CarUtils ,
inputTryCount() 처럼 무언가 입력을 받는 메소드들은 InputView 등
각 메소드들이 어떤 것에 관련되어 있는지에 따라 클래스 여러 개로 나누어서 분리해보시면 좋을 것 같아요
클래스는 반드시 어떤 실존하는 물체에 대한 것들만을 구현할 필요는 없고,
어떤 관련있는 도구들을 모아둔 것(마치 공구상자 처럼)도 클래스로 만들면 좋은 경우가 많습니다
|
|
||
| System.out.print('\n'); |
There was a problem hiding this comment.
이것도 저번에 말씀드린 부분이었던 것 같은데 반영해주셨네요...! 훌륭합니다
| private List<Car> createCars() { | ||
| System.out.println("경주할 자동차 이름을 입력하세요.(이름은 쉼표(,) 기준으로 구분)"); | ||
| String input = Console.readLine(); | ||
| String[] names = input.split(","); | ||
|
|
||
| validateNames(names); | ||
|
|
||
| List<Car> cars = new ArrayList<>(); | ||
| for (String name : names) { | ||
| cars.add(new Car(name)); | ||
| } | ||
| return cars; | ||
| } |
There was a problem hiding this comment.
전체적으로 메소드 분리 잘 진행해 주셨는데, 이 부분은 조금 분리해도 괜찮을 것 같아요!
자동차 이름을 입력받는 부분, 그리고 입력받은 문자열을 통해 자동차를 만들어주고 List 에 저장해주는 부분
이런 식으로 나누어봐도 좋을 것 같습니다
| if (used.contains(name)) { | ||
| throw new IllegalArgumentException(); | ||
| } | ||
| used.add(name); | ||
| } |
There was a problem hiding this comment.
이 부분도 이름의 중복을 검증하는 부분이니, 별도의 메소드로 분리해도 좋을 것 같아요
No description provided.